fix(gates): a capability red caused by branch isolation must say so (#34) - #43
fix(gates): a capability red caused by branch isolation must say so (#34)#43stranske wants to merge 1 commit into
Conversation
) The capability ledger is shared machine-local state; code is branch-isolated. So any branch that registers a capability turns every SIBLING branch's `verify.py` red on three gates, with a message that named only the capability id: test_capability_admission.py -> {'evidence-acquisition': ['caller_exists','heartbeat','fixture']} test_capability_set_coverage.py -> ['evidence-acquisition'] test_model_tier_resolution.py -> ['evidence-acquisition'] That is indistinguishable from the defect those gates exist to catch — a row registered with no implementation at all — and on 2026-08-22 the ambiguity cost a full misdiagnosed session. The proposed remedies for a LIVE capability were to retire its ledger row or mask it with a WAIVER; the module was on an unmerged branch the whole time, carrying a hard dependency on a `capabilities.unblock()` guard from that branch's parent, so the waiver would have hidden a latched-gate bug. The misdiagnosis rested on `git log --all` returning nothing for a branch whose ref had never been fetched, and emptiness being read as proof. DEDUP FINDING (CLAUDE.md §0), recorded before writing code. The DETECTION already existed here: `_entrypoint_files`, the `entrypoint_missing` / `entrypoint_external` defect classes, and `heartbeat_reachable`'s `no_local_entrypoint`. Verified live — the audit already reported `evidence-acquisition -> ['entrypoint_missing']`. What was absent was any route from that fact into the three failure messages, and any statement of the branch-isolation cause anywhere in the tree (`grep -rn 'unmerged|git log --all|sibling branch|branch-isolat'` over *.py/*.md: zero hits). So this WIRES and extends the existing detector; it does not add a second resolver, and it is not a new capability — it is test diagnostics inside `capability-activation-audit`, with no dispatch path, no outcome and no ledger row, exactly as `env_prereq.py` is. - `_entrypoint_declarations` factors the declaration parse out of `_entrypoint_files`, so the resolver and the diagnosis cannot drift into naming different files. It also returns the ABSENT names, which neither existing resolver did. - `entrypoint_presence` gives four states, because each demands a different action: check a branch, change another repo, fix the row, or fix the capability. - `entrypoint_diagnosis` is the one shared helper the three gates prepend. Prepended, not appended: the hand-rolled gate runners print only `str(exc)[:400]`, and a selftest pins that one capability's diagnosis fits that budget. - The audit's own `entrypoint_external` test now comes from `entrypoint_presence`, so the branch at `audit()` and the text the gates print cannot disagree about which case a capability is in. - Deliberately pure: no `git log` is executed. Running it here would report an unfetched branch as "nothing found" — the exact mistake that produced the wrong verdict. The text prints the command with the caveat instead. NOT a skip, in either case. Both reds still FAIL; the text now says which is which. Verification. `capability_activation_audit.py --selftest` covers present / absent-from-tree / another-repo / undeclared, a half-resolving `a.py/b.py`, the `->` token that names no module, the truncation budget, and the unreadable-ledger fallback. Two pytest tests guard the distinction and that all three call sites still CALL the helper (matched on `audit.entrypoint_diagnosis(`, not the bare name — every one of those files also mentions it in a comment). Deliberate break -> revert demonstrated twice: collapsing `ENTRYPOINT_ABSENT` into the present case fails both the selftest and the pytest test; deleting one call site fails the wiring guard. `test_capabilities.py`'s live-ledger write guard caught a real defect in the first draft — the diagnosis used `capabilities.load(REG)`, which reconciles AND PERSISTS. Now `load_declared`. Floor 366 -> 370, measured not assumed: `--collect-only` reports 370 and `^def test_` counts confirm exactly two added tests, so 366 was already 2 below this branch's base. No ceiling moved; both new tests build synthetic input and read source, so they need no populated ledger. Fresh-state run (`ORCH_STATE_DIR` + `ORCH_LOCAL_RUNTIME` empty, the CI equivalent): VERIFIED, 370 collected against floor 370, 0 failed. On this machine's live ledger 3 tests still fail — the pre-existing `evidence-acquisition` branch-isolation red, now self-diagnosing, and nothing else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 3 minutes Limit details: You’ve used the included review currently available. Your 69 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
Comment |
|
Closing as superseded — but not discarded. Two sessions built this diagnostic independently and in parallel, and the convergence was near-total: the same That duplication was a real dedup miss on my side, and it is worth naming: the §0 check greps the tree, the dormancy inventory and Three things here were better than what landed in #46, and all three are lifted into #51 rather than lost:
The floor bump here (368 → 370) is also in #51, measured on the merge result with ceilings untouched. 🤖 Addressed by Claude Code |
…ointer `entrypoint_presence()` with four states, same `_MODULE_NAME_RE` fix for the punctuation token, same split of `_entrypoint_files`. #46 merged first; #43 is now CONFLICTING against main. That duplication was a real dedup miss: the §0 check greps the tree, the dormancy inventory and the backlog, and neither session checked OPEN PRs. The diagnostic they both shipped tells a reader to check open PRs before retiring a row; the same habit applies before building one. This salvages the three things #43 had that #46 lacked, rather than discarding them with the PR. 1. `test_an_absent_entrypoint_diagnoses_itself_differently_from_a_real_defect` — #46 put the equivalent checks in `capability_activation_audit --selftest`, and a selftest is NOT guarded by `.verify-floor.json`. Only a COLLECTED test is, so the pytest half was the part worth keeping. 2. `test_the_capability_gates_all_consult_the_entrypoint_diagnosis` — the sharper idea, and #46 had no equivalent at all: nothing noticed if one of the three gates quietly stopped calling the helper and went back to a bare capability id. It matches the CALL, not the bare name, because all three files also MENTION the helper in a comment — a name-only grep would keep passing after someone deleted the call and left the comment. 3. The `git fetch --all && git log --all --oneline -- <module>` pointer, WITH its caveat. `git log --all` is the natural next probe and on 2026-08-22 it came back empty for a module that existed, because the branch holding it had never been fetched into that checkout. An empty result there means "not fetched", never "does not exist", so the command and its precondition travel together or the pointer reproduces the original misreading. Also adds `absent_entrypoint_report(..., ledger=)` / `absent_entrypoint_note(..., ledger=)` so both tests inject their rows instead of reading the running instance's ledger. That is why neither test skips anywhere: the interesting case is one row present and one absent, and no real ledger is reliably both. Adding a machine-independent test is the preferred way to grow this suite — see the 24 -> 26 -> 24 ceiling episode in `.verify-floor.json`. FLOOR 368 -> 370, measured on the merge result, ceilings untouched (24/7/2) and nothing new skipped. Verified with `python3 verify.py`: 370 passed, 0 failed, 0 skipped, 83/83 selftests, 43/43 can-fire, 5/5 gates. Deliberate break -> revert, each caught then reverted byte-identically: removing one gate's call to the helper while leaving the comment behind (caught by the drift guard); making a PRESENT row emit a diagnosis (caught — it would send a reader to wait for a merge of code already in front of them); dropping the fetch-first caveat from the pointer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…70) * test(diagnostics): salvage PR #43's pytest coverage and fetch-first pointer Rebuilt on current main (PR #43's branch was CONFLICTING and #46/#60 have since reshaped both files). #43 and #46 built the entrypoint-absence diagnostic independently and in parallel; #46 merged first, so this lifts the three things #43 had that #46 lacked rather than losing them with the closed PR. 1. `test_an_absent_entrypoint_diagnoses_itself_differently_from_a_real_defect` — #46 put the equivalent checks in `capability_activation_audit --selftest`, and a selftest is NOT guarded by `.verify-floor.json`. Only a COLLECTED test is, so this is the half that makes the behaviour hold. 2. `test_the_capability_gates_all_consult_the_entrypoint_diagnosis` — the sharper idea, with no equivalent in #46 at all: nothing else notices if one of the three gates quietly stops calling the helper and reverts to a bare capability id. It matches the CALL, not the bare name, because all three files also MENTION the helper in a comment — a name-only grep would keep passing after someone deleted the call and left the comment behind. 3. The `git fetch --all && git log --all --oneline -- <module>` pointer, WITH its caveat. `git log --all` is the natural next probe and on 2026-08-22 it came back empty for a module that existed, because the branch holding it had never been fetched into that checkout. An empty result there means "not fetched", never "does not exist", so the command and its precondition travel together or the pointer reproduces the misreading it exists to prevent. Also adds `absent_entrypoint_report(..., ledger=)` / `absent_entrypoint_note(..., ledger=)` so both tests inject their rows instead of reading the running instance's ledger. That is why neither test skips anywhere: the interesting case is one row present and one absent, and no real ledger is reliably both. A machine-independent test is the preferred way to grow this suite — see the 24 -> 26 -> 24 ceiling episode in `.verify-floor.json`. FLOOR 402 -> 404, measured on the merge result, and it must be exact now that #56 made `collected` an EQUALITY. No ceiling moved and nothing new is skipped. Deliberate break -> revert, each caught then reverted byte-identically: * stripped one gate's call to the helper while leaving the comment behind — the drift guard fired and named the file; * dropped the fetch-first caveat from the pointer — the pointer assertion fired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(diagnostics): two CodeRabbit findings — a short-circuit and a vacuous guard Both from the review on PR #51, both real, and the second is embarrassing in a useful way. 1. THE FETCH COMMAND OMITTED THE MODULES THAT MATTERED MOST. The module list behind `git log --all` was `{sibling hits} or {missing candidates}`. `or` short-circuits, so as soon as ONE absent row was found in a sibling checkout, the candidates of every row found NOWHERE were dropped — and those are precisely the rows the pointer exists for, because a module in no sibling checkout is the one most likely to sit on an unfetched remote branch. The command said "to check every branch" while silently omitting the hardest cases. Now a union, with the truncation counted rather than silent (`+N more module(s) not shown`), because a cap that hides modules would put the same lie back in a smaller place. 2. THE DRIFT GUARD WAS VACUOUS FOR ITS OWN FILE. It searched each whole FILE for the substring `audit.absent_entrypoint_note(` — which appears SIX times in `test_capability_set_coverage.py` alone: in the docstring, in sibling tests, and inside the assertion itself. So deleting the real call from the recurrence-fixture gate left the guard passing. A guard written specifically to catch a check that cannot fail, that itself could not fail. That is this repo's founding defect wearing the uniform of its own countermeasure. It now walks the AST of ONE NAMED FUNCTION per file, from an explicit `GATE_CALL_SITES` mapping. A string literal or a comment cannot satisfy an AST call match at all, and a RENAMED gate asserts rather than passing quietly — "some call somewhere in the file" was the whole weakness. Deliberate break -> revert, each caught then reverted byte-identically: * union -> `or`: the new regression test failed naming the exact omission (`b_lane.py` absent from the command while `a_lane.py` was present); * deleted the real call from this file's gate, leaving all six literals in place: the AST guard failed where the substring version had passed — the specific hole CodeRabbit found; * renamed a gate function: caught, with a message saying to update `GATE_CALL_SITES` deliberately. FLOOR 404 -> 405: one new test, `test_the_fetch_command_names_every_absent_module_not_just_the_found_ones`. It needs BOTH kinds of absent row present at once, which is why it is its own test rather than more assertions on an existing one — with only one kind, `or` and `|` are indistinguishable. It injects its ledger rows, so it skips nowhere. No ceiling moved. Verified with `python3 verify.py`: 405 passed, 0 failed, 0 skipped, 84/84 selftests, 43/43 can-fire, 5/5 gates. `ruff check .` and `black --check --line-length 100` both clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(diagnostics): address PR #70 CodeRabbit threads Add truncation regression for seven-plus absent modules and tighten the AST gate predicate to require audit.absent_entrypoint_note in the named function body only, ignoring nested defs and non-audit receivers. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): formatting/lint * chore(verify): raise floor to 415 on PR #70 merge result CI measured 415 collected (389 passed + 26 skipped) after merging main into the diagnostics salvage branch. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Tim Stranske <tim@stranskemo.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
) #43 -> #51 -> #70 is a salvage chain and #70 MERGED, so both predecessors read as "closed, work already landed". Checked file-by-file rather than trusting that: #70 carried `capability_activation_audit.py`, `test_capability_set_coverage.py` and `.verify-floor.json`, and `absent_entrypoint_note` / `entrypoint_presence` are on main in four files. Two pieces were NOT carried, and both are the kind of thing that reads as landed because its neighbours did. 1. #51's exp_id_template assertions (`exploration_backfill.py`). Main has `exp_id_template` in PRODUCTION code at two sites — built as `f"backfill-{_exp_id_slug(target)}"` and consumed as `f"{job.get('exp_id_template')}-{int(time.time())}"` — and ZERO selftest assertions pinning either. So the derivation `o/r#1` -> `backfill-o-r-1` and the link between the planned template and the DISPATCHED exp_id were both unasserted: a change to the slugging would silently rename every backfill experiment and surface much later as unjoinable experiment artifacts. Both halves are asserted, deliberately, because pinning the template alone would leave the two free to drift apart — the same shape as a gate whose measuring window differs from its draining window. 2. #43's FETCH-FIRST pointer (`CLAUDE.md`). #51's own title advertised it ("salvage PR #43's pytest coverage and fetch-first pointer") and #70 dropped `CLAUDE.md` entirely; main has no occurrence of "fetch" in that file. The trap is specific and cost a full session on 2026-08-22: `--all` searches the refs this checkout HAS, so `git log --all --oneline -- <file>` over an UNFETCHED sibling branch returns empty and reads as "no such file was ever committed anywhere". That false negative is what produced the wrong verdict the surrounding bullet exists to prevent, so the bullet was documenting the conclusion while omitting the step that got it wrong. DELIBERATE-BREAK -> REVERT, both reverted clean: * slug prefix `backfill-` -> `bf-`: assertion 1 fires, reporting `exp_id_template: 'bf-o-r-1'`. * dispatched id stops deriving from the template (`f"detached-{int(time.time())}"`): assertion 2 fires, reporting `exp_id: 'detached-1787524963'`. Assertion 1 still PASSES under this break, which is why both are needed rather than either alone. Verified with `python3 verify.py`: 427 passed, 0 failed, 0/26 skipped, 84/84 selftests, 43/43 can-fire, 5/5 gates. Floor untouched — both assertions live inside an existing `--selftest`, so nothing new is collected. ruff + black -l 100 clean. Co-authored-by: Tim Stranske <tim@stranskemo.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The capability ledger is shared machine-local state; code is branch-isolated. So any branch
that registers a capability turns every SIBLING branch's
verify.pyred on three gates, witha message that named only the capability id:
That is indistinguishable from the defect those gates exist to catch — a row registered with no
implementation at all — and on 2026-08-22 the ambiguity cost a full misdiagnosed session. The
proposed remedies for a LIVE capability were to retire its ledger row or mask it with a WAIVER;
the module was on an unmerged branch the whole time, carrying a hard dependency on a
capabilities.unblock()guard from that branch's parent, so the waiver would have hidden alatched-gate bug. The misdiagnosis rested on
git log --allreturning nothing for a branchwhose ref had never been fetched, and emptiness being read as proof.
DEDUP FINDING (CLAUDE.md §0), recorded before writing code. The DETECTION already existed here:
_entrypoint_files, theentrypoint_missing/entrypoint_externaldefect classes, andheartbeat_reachable'sno_local_entrypoint. Verified live — the audit already reportedevidence-acquisition -> ['entrypoint_missing']. What was absent was any route from that factinto the three failure messages, and any statement of the branch-isolation cause anywhere in the
tree (
grep -rn 'unmerged|git log --all|sibling branch|branch-isolat'over .py/.md: zerohits). So this WIRES and extends the existing detector; it does not add a second resolver, and
it is not a new capability — it is test diagnostics inside
capability-activation-audit, with nodispatch path, no outcome and no ledger row, exactly as
env_prereq.pyis._entrypoint_declarationsfactors the declaration parse out of_entrypoint_files, so theresolver and the diagnosis cannot drift into naming different files. It also returns the
ABSENT names, which neither existing resolver did.
entrypoint_presencegives four states, because each demands a different action: check abranch, change another repo, fix the row, or fix the capability.
entrypoint_diagnosisis the one shared helper the three gates prepend. Prepended, notappended: the hand-rolled gate runners print only
str(exc)[:400], and a selftest pins thatone capability's diagnosis fits that budget.
entrypoint_externaltest now comes fromentrypoint_presence, so the branchat
audit()and the text the gates print cannot disagree about which case a capability is in.git logis executed. Running it here would report an unfetched branchas "nothing found" — the exact mistake that produced the wrong verdict. The text prints the
command with the caveat instead.
NOT a skip, in either case. Both reds still FAIL; the text now says which is which.
Verification.
capability_activation_audit.py --selftestcovers present / absent-from-tree /another-repo / undeclared, a half-resolving
a.py/b.py, the->token that names no module, thetruncation budget, and the unreadable-ledger fallback. Two pytest tests guard the distinction and
that all three call sites still CALL the helper (matched on
audit.entrypoint_diagnosis(, not thebare name — every one of those files also mentions it in a comment). Deliberate break -> revert
demonstrated twice: collapsing
ENTRYPOINT_ABSENTinto the present case fails both the selftestand the pytest test; deleting one call site fails the wiring guard.
test_capabilities.py's live-ledger write guard caught a real defect in the first draft — thediagnosis used
capabilities.load(REG), which reconciles AND PERSISTS. Nowload_declared.Floor 366 -> 370, measured not assumed:
--collect-onlyreports 370 and^def test_countsconfirm exactly two added tests, so 366 was already 2 below this branch's base. No ceiling moved;
both new tests build synthetic input and read source, so they need no populated ledger.
Fresh-state run (
ORCH_STATE_DIR+ORCH_LOCAL_RUNTIMEempty, the CI equivalent): VERIFIED,370 collected against floor 370, 0 failed. On this machine's live ledger 3 tests still fail —
the pre-existing
evidence-acquisitionbranch-isolation red, now self-diagnosing, and nothingelse.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com